Skip to content

Conversation

chris-morgan
Copy link
Member

The existing description was incorrect for socket addresses, and misleading: users would see “invalid IP address syntax” and suppose they were supposed to provide an IP address rather than a socket address.

I contemplated making it two variants (IP, socket), but realised we can do still better for the IPv4 and IPv6 types, so here it is as six.

I contemplated more precise error descriptions (e.g. “invalid IPv6 socket address syntax: expected a decimal scope ID after %”), but that’s a more invasive change, and probably not worthwhile anyway.

@rust-highfive
Copy link
Contributor

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs to request review from a libs-api team reviewer. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Apr 18, 2022
@rust-highfive
Copy link
Contributor

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 18, 2022
@chris-morgan
Copy link
Member Author

r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs

(Changes observable runtime behaviour of library APIs—Error::description and Display for AddrParseError.)

@rustbot rustbot added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 18, 2022
@joshtriplett
Copy link
Member

Name bikeshedding aside, this seems like a reasonable fix to me.

The existing description was incorrect for socket addresses, and
misleading: users would see “invalid IP address syntax” and suppose they
were supposed to provide an IP address rather than a socket address.

I contemplated making it two variants (IP, socket), but realised we can
do still better for the IPv4 and IPv6 types, so here it is as six.

I contemplated more precise error descriptions (e.g. “invalid IPv6
socket address syntax: expected a decimal scope ID after %”), but that’s
a more invasive change, and probably not worthwhile anyway.
@chris-morgan chris-morgan force-pushed the AddrParseError-description-improvements branch from e0ff36f to 0255398 Compare April 19, 2022 03:04
@joshtriplett
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 19, 2022

📌 Commit 0255398 has been approved by joshtriplett

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 19, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 19, 2022
…tion-improvements, r=joshtriplett

Improve AddrParseError description

The existing description was incorrect for socket addresses, and misleading: users would see “invalid IP address syntax” and suppose they were supposed to provide an IP address rather than a socket address.

I contemplated making it two variants (IP, socket), but realised we can do still better for the IPv4 and IPv6 types, so here it is as six.

I contemplated more precise error descriptions (e.g. “invalid IPv6 socket address syntax: expected a decimal scope ID after %”), but that’s a more invasive change, and probably not worthwhile anyway.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 19, 2022
…tion-improvements, r=joshtriplett

Improve AddrParseError description

The existing description was incorrect for socket addresses, and misleading: users would see “invalid IP address syntax” and suppose they were supposed to provide an IP address rather than a socket address.

I contemplated making it two variants (IP, socket), but realised we can do still better for the IPv4 and IPv6 types, so here it is as six.

I contemplated more precise error descriptions (e.g. “invalid IPv6 socket address syntax: expected a decimal scope ID after %”), but that’s a more invasive change, and probably not worthwhile anyway.
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 20, 2022
Rollup of 6 pull requests

Successful merges:

 - rust-lang#93313 (Check if call return type is visibly uninhabited when building MIR)
 - rust-lang#96160 (Miri/interpreter debugging tweaks)
 - rust-lang#96167 (Replace sys/unix/weak AtomicUsize with AtomicPtr)
 - rust-lang#96168 (Improve AddrParseError description)
 - rust-lang#96206 (Use sys::unix::locks::futex* on wasm+atomics.)
 - rust-lang#96234 (remove_dir_all_recursive: treat ELOOP the same as ENOTDIR)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 01d4731 into rust-lang:master Apr 20, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants